Skip to content

fix(mysql): pull INVISIBLE/GIPK columns during snapshot#4553

Merged
dtunikov merged 4 commits into
mainfrom
fix/mysql-snapshot-invisible-columns
Jul 9, 2026
Merged

fix(mysql): pull INVISIBLE/GIPK columns during snapshot#4553
dtunikov merged 4 commits into
mainfrom
fix/mysql-snapshot-invisible-columns

Conversation

@dtunikov

@dtunikov dtunikov commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

MySQL/MariaDB QRep snapshot took a SELECT * fast path in buildSelectedColumns. Wildcard expansion omits INVISIBLE columns — including the generated invisible primary key (my_row_id) from sql_generate_invisible_primary_key. But the destination table (created from information_schema.columns) and CDC (binlog row images) both include those columns.

Result: snapshot rows keep destination defaults forever while CDC populates real values — silent divergence. Worst case is GIPK, where the invisible my_row_id is the table's only PK, so snapshot rows arrive with no key, breaking dedup/ordering.

Fix

Always build an explicit column list from the fetched table schema (drop the SELECT * fast path). The schema already contains invisible/GIPK columns, and naming them explicitly bypasses wildcard invisibility, so the snapshot pulls them — matching CDC and the destination DDL.

Tests

  • Unit: buildSelectedColumns never returns *; invisible/GIPK columns are listed explicitly.
  • E2E (clickhouse_mysql_test.go): Test_MySQL_Invisible_Column_Consistency and Test_MySQL_GIPK_Consistency compare snapshot vs CDC values (version-gated; GIPK is MySQL-only).

🤖 Generated with Claude Code

The SELECT * snapshot fast path relied on wildcard expansion, which omits
INVISIBLE columns (and the generated invisible primary key, my_row_id). The
destination table and CDC both include those columns, so snapshot rows kept
destination defaults forever while CDC populated real values — silent
divergence. For GIPK the invisible my_row_id is the only PK, so snapshot rows
arrived with no key, breaking dedup/ordering.

Always build an explicit column list from the fetched table schema so invisible
columns are named explicitly (bypassing wildcard invisibility). Add INVISIBLE
and GIPK e2e tests comparing snapshot and CDC.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dtunikov dtunikov requested a review from a team as a code owner July 8, 2026 16:21
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes MySQL/MariaDB QRep snapshot consistency by ensuring snapshots explicitly select all columns from the fetched schema (including INVISIBLE and generated invisible primary key columns), aligning snapshot output with destination DDL and CDC/binlog row images.

Changes:

  • Remove the SELECT * fast-path by always building an explicit column list in buildSelectedColumns.
  • Add MySQL/MariaDB minimum-version constants for invisible columns and MySQL GIPK.
  • Add/extend unit + ClickHouse E2E tests to validate snapshot vs CDC consistency for INVISIBLE and GIPK scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
flow/pkg/mysql/validation.go Adds version gates for invisible columns and generated invisible primary keys used by tests.
flow/connectors/mysql/qrep.go Removes the * selection fast-path so snapshots explicitly name columns (including INVISIBLE/GIPK).
flow/connectors/mysql/qrep_test.go Updates and extends unit coverage to ensure selected columns are explicit (no *).
flow/e2e/clickhouse_mysql_test.go Adds E2E consistency tests comparing snapshot vs CDC for INVISIBLE and GIPK columns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

}

return selectedColumns
return strings.Join(columns, ", ")
Comment on lines +2466 to +2472
require.NoError(s.t, s.source.Exec(s.t.Context(), `SET SESSION sql_generate_invisible_primary_key = ON`))
require.NoError(s.t, s.source.Exec(s.t.Context(), fmt.Sprintf(`
CREATE TABLE IF NOT EXISTS %s (
name VARCHAR(50) NOT NULL
)
`, srcFullName)))
require.NoError(s.t, s.source.Exec(s.t.Context(), `SET SESSION sql_generate_invisible_primary_key = OFF`))
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

❌ Test Failure

Analysis: The new Test_MySQL_GIPK_Consistency test — which validates the exact GIPK-snapshot feature this PR adds — hangs with the mirror "stuck in snapshot" and times out in STATUS_SETUP consistently across both MySQL 6.0 and 8.0 matrix configs, indicating a real bug in the PR's snapshot logic rather than a flaky failure.
Confidence: 0.9

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

@ilidemi ilidemi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The task will need e2e checking on how this shows up in ClickPipes UI and whether it passes validations

Comment thread flow/e2e/clickhouse_mysql_test.go Outdated
@ilidemi

ilidemi commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Oh CI doesn't pass, something's up

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

❌ 2 Tests Failed:

Tests completed Failed Passed Skipped
2998 2 2996 361
View the top 3 failed test(s) by shortest run time
github.com/PeerDB-io/peerdb/flow/connectors/postgres::TestRunPipeline_DstExitsWhileSrcWriting
Stack Traces | 0.01s run time
=== RUN   TestRunPipeline_DstExitsWhileSrcWriting
    pgdump_schema_test.go:174: expected dst failure, got src failed: exit status 141
        stderr:
--- FAIL: TestRunPipeline_DstExitsWhileSrcWriting (0.01s)
github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
Stack Traces | 0.01s run time
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH_Cluster
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH_Cluster (0.01s)
2026/07/09 09:37:18 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/09 09:37:18 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_machcl_ya1gn1nj.test_date_coercion
github.com/PeerDB-io/peerdb/flow/e2e::TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_GIPK_Consistency
Stack Traces | 61.2s run time
=== RUN   TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_GIPK_Consistency
=== PAUSE TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_GIPK_Consistency
=== CONT  TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_GIPK_Consistency
2026/07/09 09:36:17 INFO Received AWS credentials from peer for connector: ci x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN}
2026/07/09 09:36:17 INFO fetched schema x-peerdb-additional-metadata={Operation:FLOW_OPERATION_UNKNOWN} table=e2e_test_mychcl_6vika981.test_mysql_ghost_schema
    clickhouse_mysql_test.go:2485: UNEXPECTED STATUS TIMEOUT STATUS_SETUP
--- FAIL: TestPeerFlowE2ETestSuiteMySQL_CH_Cluster/Test_MySQL_GIPK_Consistency (61.18s)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔄 Flaky Test Detected

Analysis: Failures are timeout/race-based (GIPK test stuck in STATUS_SETUP past the 60s wait in both jobs, plus a SIGPIPE ordering race in TestRunPipeline_DstExitsWhileSrcWriting), all unrelated to the commit's Renovate-script change, indicating environmental flakiness rather than a real bug.
Confidence: 0.9

✅ Automatically retrying the workflow

View workflow run

@dtunikov dtunikov enabled auto-merge (squash) July 9, 2026 09:35
@dtunikov dtunikov disabled auto-merge July 9, 2026 09:45
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

❌ Test Failure

Analysis: Both mysql-gtid jobs deterministically fail the PR's own feature test (Test_MySQL_GIPK_Consistency) with an identical STATUS_SETUP timeout, indicating the GIPK/INVISIBLE snapshot code this PR introduces hangs during setup — a real bug, not a flake.
Confidence: 0.85

⚠️ This appears to be a real bug - manual intervention needed

View workflow run

@dtunikov dtunikov merged commit 65c76c7 into main Jul 9, 2026
20 of 21 checks passed
@dtunikov dtunikov deleted the fix/mysql-snapshot-invisible-columns branch July 9, 2026 10:47
@dtunikov

dtunikov commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

tested e2e with other clickpipes components, works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants